-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Narrow types based on collection containment #20602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
|
The setuptools regression is: The xarray regression is just a redefinition issue, now that we have narrowed to Literal |
| narrowable_indices={0}, | ||
| ) | ||
|
|
||
| # We only try and narrow away 'None' for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is actually not really needed anymore. I will clean it up, there is one test case affected, a later PR in my stack changes it
|
Planning on merging this in the next few days |
JukkaL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to finally have this working! Left a few comments about tests.
test-data/unit/check-narrowing.test
Outdated
| if x in typ: | ||
| reveal_type(x) # N: Revealed type is "Literal['a'] | Literal['b']" | ||
| else: | ||
| reveal_type(x) # N: Revealed type is "builtins.str" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a duplicate test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'd copied these tests from the first PR that attempted to make this change, but now I've just rewritten them completely from scratch
test-data/unit/check-narrowing.test
Outdated
| typeAlpha: Tuple[Literal['a', 'b', 'c'], ...] = ('a', 'b', 'c') | ||
| strIn: Literal['c'] = "c" | ||
| reveal_type(strIn) # N: Revealed type is "Literal['c']" | ||
| #Check we don't expand a Literal into the Union type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style nit: missing space after #.
test-data/unit/check-narrowing.test
Outdated
| [builtins fixtures/narrowing.pyi] | ||
| [typing fixtures/typing-medium.pyi] | ||
|
|
||
| [case testTypeNarrowingStringInList] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test type narrowing with an unsupported collection type (Sequence?). Test unsupported equality narrowing with collection, i.e. a case where normal equality doesn't narrow, so it shouldn't narrow in the context of a collection either (I think there was a bug like this in the past -- it's possible that there is an existing test case, though).
Can this be used for enum type narrowing, e.g. if there is a union with enum type and non-enum type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'd added a test for the consistency bug as a regression test the last time round. That is testConsistentNarrowingEqAndIn and you can see the change in this PR is desirable.
The implementation in this PR kind of guarantees that the narrowing we get from in and from == are basically the same
Yes, can be used for enum type narrowing when non-enum type is not ambiguous. I will add a test in a future PR (a later commit in my series rewrites some of those tests)
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: spark (https://github.com/apache/spark)
+ python/pyspark/pandas/utils.py:756: error: Unused "type: ignore" comment [unused-ignore]
mypy (https://github.com/python/mypy)
+ mypy/constraints.py:265: error: Argument "arg_kinds" to "Parameters" has incompatible type "list[Literal[ArgKind.ARG_POS, ArgKind.ARG_STAR, ArgKind.ARG_STAR2]]"; expected "list[ArgKind]" [arg-type]
+ mypy/constraints.py:265: note: "list" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
+ mypy/constraints.py:265: note: Consider using "Sequence" instead, which is covariant
+ mypyc/irbuild/util.py:141: error: Redundant cast to "Literal['native_class', 'allow_interpreted_subclasses', 'serializable', 'free_list_len']" [redundant-cast]
+ mypyc/irbuild/util.py:141: note: See https://mypy.rtfd.io/en/stable/_refs.html#code-redundant-cast for more info
Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/context/menu.py:144: error: Incompatible return value type (got "CommandType", expected "Literal[CommandType.USER, CommandType.MESSAGE]") [return-value]
altair (https://github.com/vega/altair)
+ tools/versioning.py:258: error: Unused "type: ignore" comment [unused-ignore]
mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/addons/core.py:172: error: Unused "type: ignore" comment [unused-ignore]
steam.py (https://github.com/Gobot1234/steam.py)
- steam/ext/csgo/state.py:213: error: Argument 1 to "__delitem__" of "dict" has incompatible type "int"; expected "AssetID" [arg-type]
- steam/ext/csgo/state.py:214: error: Argument 1 to "__delitem__" of "dict" has incompatible type "int"; expected "AssetID" [arg-type]
setuptools (https://github.com/pypa/setuptools)
+ setuptools/_shutil.py:39: error: Incompatible return value type (got "None", expected "_T") [return-value]
pandas (https://github.com/pandas-dev/pandas)
+ pandas/io/stata.py:1437: error: Redundant cast to "str" [redundant-cast]
+ pandas/io/stata.py:1797: error: Redundant cast to "str" [redundant-cast]
+ pandas/io/stata.py:1803: error: Redundant cast to "str" [redundant-cast]
discord.py (https://github.com/Rapptz/discord.py)
+ discord/components.py:1464: error: Unused "type: ignore" comment [unused-ignore]
+ discord/app_commands/models.py:900: error: Unused "type: ignore" comment [unused-ignore]
+ discord/threads.py:286: error: Unused "type: ignore" comment [unused-ignore]
+ discord/state.py:693: error: Unused "type: ignore" comment [unused-ignore]
archinstall (https://github.com/archlinux/archinstall)
+ archinstall/lib/menu/list_manager.py:84: error: Redundant cast to "str" [redundant-cast]
rotki (https://github.com/rotki/rotki)
+ rotkehlchen/types.py:1278: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/types.py:1286: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/chain/ethereum/utils.py:90: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/inquirer.py:976: error: Redundant cast to "Literal[ChainID.ETHEREUM, ChainID.POLYGON_POS, ChainID.OPTIMISM, ChainID.ARBITRUM_ONE, ChainID.GNOSIS, ChainID.BASE, ChainID.BINANCE_SC]" [redundant-cast]
+ rotkehlchen/tasks/assets.py:459: error: Unused "type: ignore[arg-type]" comment [unused-ignore]
+ rotkehlchen/chain/aggregator.py:1187: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/rotkehlchen.py:739: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/rotkehlchen.py:744: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/rotkehlchen.py:1017: error: Redundant cast to "Literal[SupportedBlockchain.ETHEREUM, SupportedBlockchain.OPTIMISM, SupportedBlockchain.POLYGON_POS, SupportedBlockchain.ARBITRUM_ONE, SupportedBlockchain.BASE, SupportedBlockchain.GNOSIS, SupportedBlockchain.SCROLL, SupportedBlockchain.BINANCE_SC]" [redundant-cast]
+ rotkehlchen/api/services/transactions.py:125: error: Unused "type: ignore" comment [unused-ignore]
+ rotkehlchen/api/services/transactions.py:126: error: "ChainManagerWithTransactions[Any]" has no attribute "node_inquirer" [attr-defined]
+ rotkehlchen/api/services/transactions.py:240: error: Redundant cast to "Literal[SupportedBlockchain.ETHEREUM, SupportedBlockchain.OPTIMISM, SupportedBlockchain.POLYGON_POS, SupportedBlockchain.ARBITRUM_ONE, SupportedBlockchain.BASE, SupportedBlockchain.GNOSIS, SupportedBlockchain.SCROLL, SupportedBlockchain.BINANCE_SC] | Literal[SupportedBlockchain.SOLANA]" [redundant-cast]
+ rotkehlchen/api/services/transactions.py:589: error: Unused "type: ignore" comment [unused-ignore]
xarray (https://github.com/pydata/xarray)
+ xarray/core/dataset.py: note: In member "pad" of class "Dataset":
+ xarray/core/dataset.py:9090: error: Incompatible types in assignment (expression has type "Literal['constant']", variable has type "Literal['edge', 'reflect', 'symmetric', 'wrap']") [assignment]
|
Test cases come from #17344 authored by Jordandev678We've wanted to do this for a long time, but previous attempt was reverted due to issues like #17841 , #17864 , #17869
Following #20492 we should now be in a position to do this narrowing
Fixes #3229
Fixes #20234
Fixes #18208
Fixes #16774